Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix Locale Picker and small picker styles #4810

Merged
merged 4 commits into from
Aug 27, 2021
Merged

fix Locale Picker and small picker styles #4810

merged 4 commits into from
Aug 27, 2021

Conversation

parasharrajat
Copy link
Member

@parasharrajat parasharrajat commented Aug 24, 2021 •
edited
Loading

Details

  1. Removed API call.
  2. Fixed the Picker style so that it shows a blue border while focused.
  3. Fixed picker style on Firefox browsers. https://expensify.slack.com/archives/C01GTK53T8Q/p1629962984011400

Fixed Issues

$ #4513

Tests | QA Steps

  1. Open the login page & sign in with some account.
  2. Sign out of the app.
  3. Switch a language.
  4. Language should change.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web | Desktop

locale-wd

Mobile Web

locale-m

iOS

Android

locale-a

@parasharrajat parasharrajat marked this pull request as ready for review August 26, 2021 05:02
@parasharrajat parasharrajat requested a review from a team as a code owner August 26, 2021 05:02
@MelvinBot MelvinBot requested review from nkuoch and removed request for a team August 26, 2021 05:02
@parasharrajat
Copy link
Member Author

cc: @Beamanator as you are assigned Engineer to the issue.

@Beamanator Beamanator requested review from Beamanator and removed request for nkuoch August 26, 2021 13:54
Beamanator
Beamanator reviewed Aug 26, 2021
View reviewed changes
Copy link
Contributor

@Beamanator Beamanator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed API call.

  • 👍 looks great, no extra, useless API calls
    Fixed the Picker style so that it shows a blue border while focused.
  • 👍 confirmed on web
    Fixed picker style on Firefox browsers. https://expensify.slack.com/archives/C01GTK53T8Q/p1629962984011400
  • 👍 looking good on Preferences page & elsewhere! Still looks good on other browsers too

A few code comments though

@parasharrajat
Copy link
Member Author

Sorry for the extra code, I have fixed that.

@Beamanator Beamanator self-requested a review August 27, 2021 07:49
@Beamanator
Copy link
Contributor

Resolved a few comments & responded to 2 more 👍
Hopefully if you add 1 more commit, the CLA assistant will be happy. If not, we'll ping someone to help (since I'm sure you already assigned the CLA)

@parasharrajat
Copy link
Member Author

Updated.

Beamanator
Beamanator approved these changes Aug 27, 2021
View reviewed changes
Copy link
Contributor

@Beamanator Beamanator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@Beamanator Beamanator merged commit 1f332a6 into Expensify:main Aug 27, 2021
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify OSBotify mentioned this pull request Aug 27, 2021
Closed
@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @Beamanator in version: 1.0.88-3 🚀

platform result
🤖 android 🤖 cancelled 🔪
🖥 desktop 🖥 cancelled 🔪
🍎 iOS 🍎 cancelled 🔪
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

OSBotify commented Sep 1, 2021

🚀 Deployed to production by @roryabraham in version: 1.0.90-2 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 failure ❌
🕸 web 🕸 success ✅

@shawnborton shawnborton mentioned this pull request Sep 1, 2021
Closed
@shawnborton
Copy link
Contributor

@parasharrajat just double checking here - what does the picker look like when it is not active? It seems like the styles are still a little off - I think when the picker is active, the border color should change instead of an additional blue border being added. Let me know if that makes sense.

@parasharrajat
Copy link
Member Author

When picker is not active it shows gray border like other input in app. There are two borders arround the picker. I changed the colour or outer border but didn't know that there were two borders. So In a follow up I will remove the inner border.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Beamanator Beamanator Beamanator approved these changes

Assignees

@parasharrajat parasharrajat

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@parasharrajat @Beamanator @OSBotify @shawnborton